Skip to content

hooks: strip heredoc bodies before dangerous-command scan#101

Open
truffle-dev wants to merge 1 commit intoghostwright:mainfrom
truffle-dev:fix/dangerous-command-blocker-heredoc-100
Open

hooks: strip heredoc bodies before dangerous-command scan#101
truffle-dev wants to merge 1 commit intoghostwright:mainfrom
truffle-dev:fix/dangerous-command-blocker-heredoc-100

Conversation

@truffle-dev
Copy link
Copy Markdown

Closes #100.

Problem

The createDangerousCommandBlocker hook regex-matches against the
raw command string. Bodies of bash heredocs are data, not commands,
so a journal entry, repro, or doc snippet that quotes a forbidden
phrase like git push --force or docker compose down was tripping
the blocker and refusing benign Bash invocations:

cat > note.md <<EOF
Reminder: avoid git push --force on shared branches.
EOF

The agent had no way to write the prose without the blocker firing.

Fix

This is option 2 from #100: strip heredoc bodies before the
pattern loop. No new dependency, no shell parser.

function stripHeredocBodies(command: string): string {
    return command.replace(
        /<<-?\s*['"]?(\w+)['"]?\n[\s\S]*?\n[ \t]*\1\s*$/gm,
        "",
    );
}

Handles <<DELIM, <<-DELIM, <<"DELIM", <<'DELIM'. The [ \t]*
before the closing delimiter covers <<- heredocs where bash strips
leading tabs from the terminator line. A real destructive command
outside the heredoc still matches against the surrounding command
text that the regex leaves intact.

Tests

Four new cases in src/agent/__tests__/hooks.test.ts:

Case Expected Why
Heredoc body that says git push --force is forbidden allow data, not command
<<'NODEEOF' body that says the docker-compose teardown phrase allow quoted-delim heredoc
Heredoc + trailing git push --force origin main block real command outside
<<-EOF\n\tbody\n\tEOF\nrm -rf / block dash-stripped + real command

Stash-bisect (stashing only src/agent/hooks.ts, leaving the new
tests in place) shows 13/2 fail without the fix, 15/0 pass with it.

$ git stash -- src/agent/hooks.ts
$ bun test src/agent/__tests__/hooks.test.ts
 13 pass
 2 fail   # the two new "allows heredoc body..." cases

$ git stash pop
$ bun test src/agent/__tests__/hooks.test.ts
 15 pass
 0 fail

What this leaves on the table

Echo/printf false positives still exist, e.g.
echo 'git push --force is bad' would still trip the blocker. That
needs option 1 from #100 (shell-token-aware scan with
shell-quote or similar), which is a larger PR. This narrow patch
addresses the highest-frequency case the agent actually hits.

Verification

  • bun test src/agent/__tests__/hooks.test.ts — 15/15 pass
  • bun run typecheck — clean
  • bun run lint — clean (formatter normalized the new strings to one line each)
  • Full suite — same 9 pre-existing failures as origin/main (Resend domain, env-pollution between provider tests); none touched by this PR

Closes ghostwright#100.

The dangerous-command blocker regex-matches against the raw `command`
string. Bodies of bash heredocs (cat > note.md <<EOF ... EOF) are data,
not commands, so a journal entry or repro that quotes a forbidden
phrase like "git push --force" or the docker-compose teardown invocation
was tripping the blocker and refusing benign Bash invocations.

Strip heredoc bodies from the scannable text before the pattern loop:

  /<<-?\s*['"]?(\w+)['"]?\n[\s\S]*?\n[ \t]*\1\s*$/gm

Handles `<<DELIM`, `<<-DELIM`, `<<"DELIM"`, `<<'DELIM'`. The `[ \t]*`
before the closing delimiter covers `<<-` heredocs where bash strips
leading tabs from the terminator line. Real destructive commands
outside the heredoc still match (verified by test).

Tests added (hooks.test.ts):
- allows heredoc body that names a forbidden phrase as data
- allows heredoc with quoted delimiter and dangerous-looking body
- blocks real dangerous command outside the heredoc
- blocks dash-stripped heredoc opener with real dangerous trailing command

Stash-bisect: 13/2 fail without the fix, 15/0 pass with the fix.

This is option 2 from ghostwright#100 (heredoc-strip, no new dep). Echo/printf
false-positives still exist; the long-term option 1 (shell-token-aware
scan) is left for a future, larger PR. This narrow patch addresses the
highest-frequency case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hooks: dangerous-command blocker matches phrases in heredoc/echo content, blocking benign Bash

1 participant